From 02e76a7e8cb4a3156cb31279bcef834bd3eb7a5d Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Wed, 11 Mar 2026 11:22:23 -0300 Subject: [PATCH] [PATCH] src: handle NGHTTP2_ERR_FLOW_CONTROL error code Refs: https://hackerone.com/reports/3531737 PR-URL: https://github.com/nodejs-private/node-private/pull/832 CVE-ID: CVE-2026-21714 Gbp-Pq: Topic sec Gbp-Pq: Name 55-handle-NGHTTP2_ERR_FLOW_CONTROL-error-code.patch --- src/node_http2.cc | 6 ++ .../test-http2-window-update-overflow.js | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 test/parallel/test-http2-window-update-overflow.js diff --git a/src/node_http2.cc b/src/node_http2.cc index ba01b6eab..9a0c264ea 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1116,8 +1116,14 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle, // The GOAWAY frame includes an error code that indicates the type of error" // The GOAWAY frame is already sent by nghttp2. We emit the error // to liberate the Http2Session to destroy. + // + // ERR_FLOW_CONTROL: A WINDOW_UPDATE on stream 0 pushed the connection-level + // flow control window past 2^31-1. nghttp2 sends GOAWAY internally but + // without propagating this error the Http2Session would never be destroyed, + // causing a memory leak. if (nghttp2_is_fatal(lib_error_code) || lib_error_code == NGHTTP2_ERR_STREAM_CLOSED || + lib_error_code == NGHTTP2_ERR_FLOW_CONTROL || lib_error_code == NGHTTP2_ERR_PROTO) { Environment* env = session->env(); Isolate* isolate = env->isolate(); diff --git a/test/parallel/test-http2-window-update-overflow.js b/test/parallel/test-http2-window-update-overflow.js new file mode 100644 index 000000000..41488af9b --- /dev/null +++ b/test/parallel/test-http2-window-update-overflow.js @@ -0,0 +1,84 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const http2 = require('http2'); +const net = require('net'); + +// Regression test: a connection-level WINDOW_UPDATE that causes the flow +// control window to exceed 2^31-1 must destroy the Http2Session (not leak it). +// +// nghttp2 responds with GOAWAY(FLOW_CONTROL_ERROR) internally but previously +// Node's OnInvalidFrame callback only propagated errors for +// NGHTTP2_ERR_STREAM_CLOSED and NGHTTP2_ERR_PROTO. The missing +// NGHTTP2_ERR_FLOW_CONTROL case left the session unreachable after the GOAWAY, +// causing a memory leak. + +const server = http2.createServer(); + +server.on('session', common.mustCall((session) => { + session.on('error', common.mustCall()); + session.on('close', common.mustCall(() => server.close())); +})); + +server.listen(0, common.mustCall(() => { + const conn = net.connect({ + port: server.address().port, + allowHalfOpen: true, + }); + + // HTTP/2 client connection preface. + conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'); + + // Empty SETTINGS frame (9-byte header, 0-byte payload). + const settingsFrame = Buffer.alloc(9); + settingsFrame[3] = 0x04; // type: SETTINGS + conn.write(settingsFrame); + + let inbuf = Buffer.alloc(0); + let state = 'settingsHeader'; + let settingsFrameLength; + + conn.on('data', (chunk) => { + inbuf = Buffer.concat([inbuf, chunk]); + + switch (state) { + case 'settingsHeader': + if (inbuf.length < 9) return; + settingsFrameLength = inbuf.readUIntBE(0, 3); + inbuf = inbuf.slice(9); + state = 'readingSettings'; + // Fallthrough + case 'readingSettings': { + if (inbuf.length < settingsFrameLength) return; + inbuf = inbuf.slice(settingsFrameLength); + state = 'done'; + + // ACK the server SETTINGS. + const ack = Buffer.alloc(9); + ack[3] = 0x04; // type: SETTINGS + ack[4] = 0x01; // flag: ACK + conn.write(ack); + + // WINDOW_UPDATE on stream 0 (connection level) with increment 2^31-1. + // Default connection window is 65535, so the new total would be + // 65535 + 2147483647 = 2147549182 > 2^31-1, triggering + // NGHTTP2_ERR_FLOW_CONTROL inside nghttp2. + const windowUpdate = Buffer.alloc(13); + windowUpdate.writeUIntBE(4, 0, 3); // length = 4 + windowUpdate[3] = 0x08; // type: WINDOW_UPDATE + windowUpdate[4] = 0x00; // flags: none + windowUpdate.writeUIntBE(0, 5, 4); // stream id: 0 + windowUpdate.writeUIntBE(0x7FFFFFFF, 9, 4); // increment: 2^31-1 + conn.write(windowUpdate); + } + } + }); + + // The server must close the connection after sending GOAWAY. + conn.on('end', common.mustCall(() => conn.end())); + conn.on('close', common.mustCall()); +})); -- 2.30.2